Skip to content

Implement solution#1802

Open
whomngmnt wants to merge 3 commits into
mate-academy:masterfrom
whomngmnt:develop
Open

Implement solution#1802
whomngmnt wants to merge 3 commits into
mate-academy:masterfrom
whomngmnt:develop

Conversation

@whomngmnt

@whomngmnt whomngmnt commented Jun 3, 2026

Copy link
Copy Markdown

[DEMO LINK] https://whomngmnt.github.io/react_people-table-advanced/

i have really problems with demo link(doesn't work).maybe you can check without it?

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Your implementation successfully meets all core task requirements. The filtering functionality (name query, sex, centuries) and sorting (name, sex, born, died with three-click cycle: ascending → descending → disabled) are correctly implemented using URL Search Params. The CenturyFilter properly uses getAll for retrieval and append for adding multiple century values. Search parameters are correctly preserved when navigating within the People page via Navbar and PersonLink components. The sidebar with filters only appears when people are loaded. The noted issues (demo link not working and missing workflow file) are infrastructure/deployment concerns rather than code implementation problems—your actual implementation code is sound. All checklist requirements are satisfied. Congratulations on the solid implementation!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

@dothearts dothearts left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

Demo Link not working (404 in console, white screen)
Please fix it and make the request.

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants